-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(webdriverio): fix types between v8 and <v8 #962
Conversation
@@ -59,7 +59,7 @@ jobs: | |||
- run: npm ci | |||
# HACK: Force a TTY to enable browser-driver-manager to manipulate stdout. | |||
- shell: 'script -q -e -c "bash {0}"' | |||
run: npx browser-driver-manager install chromedriver --verbose | |||
run: npx browser-driver-manager install chromedriver=119 --verbose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temporary fix that will be fixed in a separate pr to fix the cli tests with 120 #963.
@@ -246,15 +253,15 @@ export default class AxeBuilder { | |||
// ensure we fail quickly if an iframe cannot be loaded (instead of waiting | |||
// the default length of 30 seconds) | |||
const { pageLoad } = await this.client.getTimeouts(); | |||
this.client.setTimeout({ | |||
(this.client as WebdriverIO.Browser).setTimeout({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes a problem with older versions of webdriverio as in newer versions they declare the function with the first parameter as this: Webdriverio.Browser.
context?: SerialContextObject, | ||
options?: RunOptions | ||
): Promise<PartialResult> => { | ||
return promisify( | ||
client | ||
.executeAsync<string, []>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older versions of webdriverio don't declare the type of the return so we can't declare it either. This defaults to <unknown, []>
for the type in newer versions which is why we need to make the change below to make the return type unknown
and then cast it to a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, just a few minor comments inline -
Co-authored-by: Gabe <41127686+Zidious@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This change brings the types from webdriverio v8 and makes them compatible with the types from <= v7. This change also prepares us to take the latest webdriverio v8 type changes which were causing our builds to break.
This also upgrades the version of
@wdio
packages intest/wdio
in order to not have multiple copies across packages. We should update all these types together so they don't go out-of-sync.QA notes:
With this change we should be able to run
@axe-core/webdriverio
package in webdriverio v5+. Using the following code, install each version of webdriverio from v5 to v8, and then compile and run using typescript to verify workingAdditional notes:
Webdriverio v7 cannot run in node v18/20, so you'll need to use node v16 in order to test <=v7. For webdriverio v5, the code above should compile but may not open a browser and run. This is a set up issue with the webdriverio and not our code so we are OK if the builds but doesn't run in v5.
Closes: #829